Skip to content

refactor(multiple): eliminate usages of any type (batch 2) #30613

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

jamOne-
Copy link
Contributor

@jamOne- jamOne- commented Mar 10, 2025

@angular-robot angular-robot bot added the area: build & ci Related the build and CI infrastructure of the project label Mar 28, 2025
@jamOne- jamOne- marked this pull request as ready for review April 11, 2025 12:26
@jamOne- jamOne- requested a review from a team as a code owner April 11, 2025 12:26
@jamOne- jamOne- requested review from mmalerba and wagnermaciel and removed request for a team April 11, 2025 12:26
@@ -235,7 +235,7 @@ export class DateFnsAdapter extends DateAdapter<Date, Locale> {
return super.deserialize(value);
}

isDateInstance(obj: any): boolean {
isDateInstance(obj: unknown): obj is Date {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should keep this as boolean even though date-fns says it's returning value is Date. Their implementation is clearly returning a boolean, not a Date-like object as it says here: https://github.com/date-fns/date-fns/blob/313b902b9a72c64501074db9bc2b9897d2db5140/src/isDate/index.ts#L33

In their original javascript implementation, they document that it's returning a boolean too`. I think this might have been a mistake when they changed to TypeScript? https://github.com/date-fns/date-fns/blob/6ba1acc644a444092c672d0d0af33fbe8e7b9481/src/isDate/index.js

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isDateInstance(obj: unknown): obj is Date indicates that isDateInstance is a "type guard". This in practice means that the function returns a boolean value, but also it narrows down the type of the passed argument (obj in this case).

If we had code like

function isFoo(x: unknown): x is Foo { ... }

const test: Foo | Bar = ...;

if (isFoo(test)) {
  // test is of type Foo here
} else {
  // test is of type Bar here
}

https://www.typescriptlang.org/docs/handbook/advanced-types.html#user-defined-type-guards.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ohh okay interesting, thanks for explaining!

@@ -263,7 +263,7 @@ export class LuxonDateAdapter extends DateAdapter<LuxonDateTime> {
return super.deserialize(value);
}

isDateInstance(obj: any): boolean {
isDateInstance(obj: unknown): obj is LuxonDateTime {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here - isn't this definitely a boolean?

@mmalerba mmalerba removed their request for review April 14, 2025 21:36
@jamOne- jamOne- requested a review from andrewseguin April 18, 2025 08:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: build & ci Related the build and CI infrastructure of the project area: google-maps area: youtube-player
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants